Skip to content

Add Nutils participants for turek-hron-fsi3 #648

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

gertjanvanzwieten
Copy link
Collaborator

This PR adds two Nutils participants for the turek-hron-fsi3 tutorial. The scripts are intentionally structured similar to our monolithic example so they can be compared side by side with https://examples.nutils.org/official-turek/.

Some notes:

  • The scripts are only tested in mutual coupling, so I don't know how they hold up with other participants. In particular I hope that traction is exchanged with the correct sign.
  • The script requires Nutils 9, which is the currently stable version.
  • The time step is set equal to the max_time_step_size, rather than the script having its own notion of a preferred time step. I'm not sure if that is idiomatic.

I hope you will find this a useful addition, let me know if anything needs changing!

@gertjanvanzwieten
Copy link
Collaborator Author

I ran both scripts through autopep8 -i but the style check is still failing. Please advise :)

@MakisH
Copy link
Member

MakisH commented Apr 28, 2025

I ran both scripts through autopep8 -i but the style check is still failing. Please advise :)

Thanks for the PR, @gertjanvanzwieten!
Maybe this is a version issue. We are using a pre-commit hook, which you could install to get the same version. This should not mess with the rest of your system.

@gertjanvanzwieten
Copy link
Collaborator Author

For some reason actions fail to kick in but I think the scripts are compliant now, changes are generated with pre-commit run.

@MakisH MakisH requested a review from uekerman July 17, 2025 14:19
Copy link
Member

@uekerman uekerman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @gertjanvanzwieten for this nice and clean contribution ❤️ and sorry for the very late reply.

Could you please tick the box in the PR to allow edits from maintainers?

The scripts are only tested in mutual coupling, so I don't know how they hold up with other participants

For how long did you run the simulation. Only the first few timesteps or till the end? If the latter, did you get identical results compared to the monolithic one?

I was able to start Nutils-Nutils and the first few time steps converged. Now I am running OpenFOAM-Nutils for longer to check physical results.

TODO (for us):

  • add participants to README

Comment on lines +272 to +273
cons['d'][r_where] = participant.read_data(
r_name, 'Displacement', r_ids, timestep)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to confirm: You are using Newmark-beta for time integration. This means, you want to read at the end of the time step size, right? This is what is done right now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, everything is solved for the end of the time step. That's what I have come to really like about Newmark. Is it a poor fit for precice?

Comment on lines +128 to +129
@dataclass
class DummyParticipant:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you use this mock for testing? Is it still required?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is not required anymore, apologies for the sloppiness (I thought I had removed it). I used it during development to validate the individual participants against the CFD and CSM tests of the original turek paper.

Comment on lines +236 to +237
args['traction'] = participant.read_data(
rw_name, 'Stress', rw_ids, timestep)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again reading at the end of the time step. Is this what you want here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my answer above.

@uekerman
Copy link
Member

fluid-openfoam coupled to solid-nutils gives correct results and runs fast enough.
Screenshot from 2025-07-30 09-21-39

@uekerman
Copy link
Member

@gertjanvanzwieten For the fluid-nutils solver, you start from zero initial values, but directly apply the full inflow velocity, correct? In OpenFOAM, we use a cos ramp over the first two seconds, 1 - cos((pi / 2) * t)) / 2 for the inflow velocity. Normally, sth like this is needed to get the partitioned simulation to run. Currently, fluid-nutils coupled to solid-dealii crashes.
Could you please add such a ramp?

@gertjanvanzwieten
Copy link
Collaborator Author

@gertjanvanzwieten For the fluid-nutils solver, you start from zero initial values, but directly apply the full inflow velocity, correct? In OpenFOAM, we use a cos ramp over the first two seconds, 1 - cos((pi / 2) * t)) / 2 for the inflow velocity. Normally, sth like this is needed to get the partitioned simulation to run. Currently, fluid-nutils coupled to solid-dealii crashes. Could you please add such a ramp?

The nutils solver applies the same ramp, see Dynamic.ramp_up on line 103 and its application on line 274. Does the solver fail in the very first time step? Could you try setting Dynamic.gamma to 1.0 and Dynamic.beta to 0.5 to make the fluid solver fully implicit?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants